Skip to content

Conversation

@timuralchin
Copy link
Contributor

No description provided.

@timuralchin timuralchin requested a review from r-abzalov March 23, 2022 07:57
@timuralchin timuralchin self-assigned this Mar 23, 2022
"""Base union type."""

class Meta:
types = (None,)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Наверное, стоит добавить проверку, что здесь лежат только DjangoObjectType в методе init_subclass_with_meta

# replace description from interface if it is not none.
if field_description.description:
merged_fields.get(field_name).description = field_description.description
merged_fields.get(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можешь плз поправить везде переносы

from tests.django_setup.models import Article, Author, Post


class PostType(DjangoObjectType):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут для тестов, наверное, лучше стоит использовать наш BaseDjangoObjectType

@classmethod
def resolve_type(cls, instance, info):
"""Resolving grapql type by models instance."""
for type in cls._meta.types:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здесь бы переименовать на type_, чтобы не перезаписывать внутреннюю функцию python-а

default_resolver = None


class BaseUnion(graphene.Union):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут бы тоже переименовать, что это DjangoModelUnion

@timuralchin timuralchin requested a review from r-abzalov March 23, 2022 13:25
@r-abzalov r-abzalov closed this Jul 5, 2022
@r-abzalov r-abzalov reopened this Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants